fix: early return for module file error in AST parsing #4151
Conversation
| @@ -0,0 +1,8 @@ | |||
| struct S { | |||
There was a problem hiding this comment.
Not sure on the exact decorators needed here, or if this test should go into the xfail directory. The test is expected to fail compilation, just with an error message instead of an ICE.
There was a problem hiding this comment.
grep for dg-error in the testsuite directory -- that should give you some nice examples for expecting error messages
0xllx0
force-pushed
the
fix/issue-4145
branch
from
23c234f to
7e5d59c
Compare
0xllx0
force-pushed
the
fix/issue-4145
branch
from
7e5d59c to
4abdf2e
Compare
CohenArthur
left a comment
CohenArthur
left a comment
There was a problem hiding this comment.
one tiny nitpick but the changes are really good, thank you for the work!
0xllx0
force-pushed
the
fix/issue-4145
branch
3 times, most recently
from
98c8cc2 to
b84e8ac
Compare
|
You need to fixup the regressions here those tests probably emit a new error now and they need updated accordingly |
0xllx0
force-pushed
the
fix/issue-4145
branch
from
b84e8ac to
edcfcfb
Compare
I rebased on the latest master ( |
| struct S { | ||
| field: [u8; { | ||
| #[path = "outer/inner.rs"] | ||
| // { dg-error "error handling module file for .inner." "#4145" { xfail *-*-* } .-1 } |
There was a problem hiding this comment.
I think .-1 should be .1, if you want to catch an error on the next line instead of the previous line
There was a problem hiding this comment.
Thanks, I changed it to .+1, and changed xfail to target since the compiler successfully returns an error (opposed to an ICE). Please let me know if that logic makes sense to you.
0xllx0
force-pushed
the
fix/issue-4145
branch
4 times, most recently
from
4559576 to
b388091
Compare
I can't remember the state of that PR when you first submitted it. This message seems to be out of date (there are multiple tests that are not passing anymore). Do you struggle with it ? Do you need some help ? You probably need more constraints over your condition. Try executing the test using GDB to understand why it emits an error. Note that the tests may also have a slight chance of being wrong, even though I doubt it. |
0xllx0
force-pushed
the
fix/issue-4145
branch
from
b388091 to
de64fde
Compare
I don't know enough at this point about the internals of Not sure what the best way forward is.
Possibly, but I currently lack the appropriate context for what the test condition should be that captures the motivating edge-case, and doesn't break the rest of the test suite. It will take me some time to get more familiar with the surrounding code.
Thanks, that will probably help trace the relevant code paths. |
Converts an assert into an early return during AST parsing. Resolves: Rust-GCC#4145 gcc/rust/ChangeLog: * ast/rust-ast.cc (Module::process_file_path): empty module early return Signed-off-by: Elle Rhumsaa <elle@weathered-steel.dev>
gcc/testsuite/ChangeLog: * rust/compile/issue-4145.rs: New test. Signed-off-by: Elle Rhumsaa <elle@weathered-steel.dev>
Thanks for mentioning this, because it made me review the condition for the early return. Somehow, I inverted the boolean condition at some point. It's fixed now. |
5 participants
Thank you for making Rust GCC better!
If your PR fixes an issue, you can add "Fixes #issue_number" into this
PR description and the git commit message. This way the issue will be
automatically closed when your PR is merged. If your change addresses
an issue but does not fully fix it please mark it as "Addresses #issue_number"
in the git commit message.
Here is a checklist to help you with your PR.
make check-rustpasses locallyclang-formatgcc/testsuite/rust/Note that you can skip the above if you are just opening a WIP PR in
order to get feedback.
Converts an assert into an early return during AST parsing.
Resolves: #4145